Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes and Improvement parsing #84

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

xZetsubou
Copy link
Contributor

Brief:

  • Fix TV Year sometimes pulled incorrectly. happens while testing on "My Hero Academia s7"
  • Fix the file name both in temp dir or media dir.
  • Fix the missing subtitle file extension.
  • Fix selecting the incorrect episode if the downloaded archive has multi subs.
  • Fix Auto download not working for after the first selected episode on play list. "e.g. play from here, a4k were only works for 1st media."
  • Fix subtitle unable to import due to the filename contains "illegal characters"
  • Improve episodes determined.
  • Improve results parsing.
  • Now auto-download will also copy the subtitle next to the video or to a custom location, depending on Kodi's subtitle storage mode path.
  • SubSource: now will also pull series that in "absolute order" due to website has absolute order for some animes?.
  • SubSource: Fix an issue of duplicated sub id yet different names. this can happen if subtitle has multi release names.

Names :

Subtitle Name:
Before:
S01E04 - 004.ar.[ReDEJA] Nige Jouzu no Wakagimi - 04

After:
[ReDEJA] Nige Jouzu no Wakagimi - 04.ara.ass

=====

Media Path:
Before: 
S01E04 - 004.ar

After:
S01E04 - 004.ar.ass

Subittles in archive:

As for the selected episode I noticed that a4ksub always fails to match the subtitle in multi subs archive file, after double check it seems it rely on "episodeid" to get the targeted sub however, none of the services pass the param. I added the episodeid param as "meta.episode" to the services action_args, I think this is good since meta episode is the only way for us to know what episode we are looking for.

Another issue i bumped into even if I passed the episode number sometimes it pull incorrect because some files name adds "S01_E02" which will take it since "01" is in the name. I couldn't find anyway to handle this other than introducing a function to extract the episode number and season this also used to improve the results parsing.

^ this has been tested with a file of "64" subtitles and works as expected.

Incorrect TV Year:

TV Year was overwritten in "__update_info_from_imdb" by "meta year" however the tv year already pulled correctly by "__scrape_imdb_id".
So I add an if statement in "__update_info_from_imdb" should set tv year as year only if it's is missing.

Results Parse Improvements:

This has been improved due to "auto download subtitle" pull the incorrect one.

Results parsing has been improved and now it even handle tv series way better.

Now subtitle and meta name will be matched by there names parts to detect the closest one, however if an incorrect episode has been detected it will be moved bottom. now this also helps alot on subtitles that named on absolute order.

These changes affect series more then movies.

Here is some test before and after of the results:

Movies // TV Series -- Results Tests

Local: Detective Conan EP 1119

series_conan

====

Local: One piece New Episode 1119

series_onpiece_newep

====

Plugin: One Piece OLD Episode 885

series_onpiece_oldep

====

Plugin: The Sims S35 E18

series_the_sims

====

Plugin: the lord of the rings the rings of power S2 E6

series_TLOTR

====

Plugin: Interstellar

move_interstellar

====

Plugin: Neir S1 E15

series_neir

===

Plugin: King of tower S1 E1

series_kot

===

Plugin: King of tower S2 E2

series_mashle

note: might be noticed but it was intended to prefer full season pack over one episode, I was gonna add this as an option in settings but at the end this worked well.

This worked for me pretty good for both movies and tv series/animes, not sure if I missed something but the more people to test it the faster to know 😅

@xZetsubou xZetsubou requested a review from newt-sc as a code owner September 29, 2024 09:20
@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch from bc44bfb to e01a29d Compare September 30, 2024 06:03
a4kSubtitles/download.py Outdated Show resolved Hide resolved
a4kSubtitles/download.py Outdated Show resolved Hide resolved
a4kSubtitles/download.py Outdated Show resolved Hide resolved
a4kSubtitles/service.py Outdated Show resolved Hide resolved
a4kSubtitles/search.py Outdated Show resolved Hide resolved
a4kSubtitles/search.py Outdated Show resolved Hide resolved
a4kSubtitles/search.py Outdated Show resolved Hide resolved
@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch 2 times, most recently from 6e815fd to fd0c92c Compare October 1, 2024 10:17
@xZetsubou
Copy link
Contributor Author

Update:
extract_season_episode now will return DictAsObject and it will also parse episodes range e.g. "ep 1 ~ 12" this improved. double check "sorter".

@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch 4 times, most recently from 6ee114e to de790ee Compare October 1, 2024 10:27
a4kSubtitles/lib/utils.py Outdated Show resolved Hide resolved
@xZetsubou
Copy link
Contributor Author

xZetsubou commented Oct 1, 2024

When I run the episodes from favorites the episode/season still empty. updated video/__get_basic_info to use extract_season_episode instead

note: extract_season_episode season and episode returns empty string instead of None, and added zfill param

@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch 4 times, most recently from bb38d68 to 38e4501 Compare October 2, 2024 11:44
@newt-sc
Copy link
Collaborator

newt-sc commented Oct 2, 2024

Rebase on latest master in order for opensubtitles tests to pass.

@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch from 38e4501 to a484a36 Compare October 2, 2024 22:13
@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch from 7c4bae2 to 9389595 Compare October 3, 2024 03:33
@xZetsubou
Copy link
Contributor Author

xZetsubou commented Oct 3, 2024

It seems a4ksub not always detect if video is playing with the logic getCondVisibility('VideoPlayer.Content())

In services.py is there a reason preventing us from using xbmc.Player().isPlayingVideo() instead since it returns correctly if video is playing or not, has_video_duration always works but VideoPlayer.Content doesn't, If possible to adjust this before merge.

has_video = (core.kodi.xbmc.getCondVisibility('VideoPlayer.Content(movies)')
or core.kodi.xbmc.getCondVisibility('VideoPlayer.Content(episodes)'))

@newt-sc
Copy link
Collaborator

newt-sc commented Oct 3, 2024

@xZetsubou

It seems a4ksub not always detect if video is playing with the logic getCondVisibility('VideoPlayer.Content())

I'm unaware of such cases. Could you give me an example for me to verify?
But in any case, there are no reasons not to switch it if the other one works better.

@xZetsubou
Copy link
Contributor Author

xZetsubou commented Oct 3, 2024

While testing I noticed when I add an episode to my favorites auto-download doesn't works, I think it's misbehaved from plugin or Kodi not sure.


Play the episode from favorites

{
    'Filename And Path': 'plugin://plugin.video.otaku/play/170695/5/', 
    'Has Duration': True, 
    'VideoPlayer.Content(movies)': False, 
    'VideoPlayer.Content(episodes)': False, 
    'isPlayingVideo': True
}

Playing the same episode from plugin

{
    'Filename And Path': 'plugin://plugin.video.otaku/play/170695/5/', 
    'Has Duration': True, 
    'VideoPlayer.Content(movies)': False, 
    'VideoPlayer.Content(episodes)': True, 
    'isPlayingVideo': True
}

@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch 2 times, most recently from f8d6d17 to cc7e97b Compare October 4, 2024 09:35
@xZetsubou

This comment was marked as outdated.

@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch 2 times, most recently from bc93eda to 6f4b2ea Compare October 5, 2024 00:33
@newt-sc
Copy link
Collaborator

newt-sc commented Oct 6, 2024

@xZetsubou Look into the failing tests, so we can merge this. They may to be related to recent changes as previously all except OpenSubtitle tests were passing.

@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch 3 times, most recently from 641814c to e967944 Compare October 7, 2024 22:19
@xZetsubou
Copy link
Contributor Author

Tests has been modified for new changes, video playing tests has been added I think now only opensubtitles with missing imdb are failing.

@newt-sc
Copy link
Collaborator

newt-sc commented Oct 7, 2024

Something is wrong with the commit, it fails to checkout. Could you push again.

@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch from e967944 to 0f91f60 Compare October 7, 2024 22:58
@xZetsubou
Copy link
Contributor Author

xZetsubou commented Oct 7, 2024

Could you push again

Done.

@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch from 0f91f60 to b7dfdcc Compare October 9, 2024 06:23
@xZetsubou
Copy link
Contributor Author

xZetsubou commented Oct 9, 2024

@newt-sc The PR is now ready and should pass the tests.

@xZetsubou xZetsubou force-pushed the fix_subs_name_multi_subs branch from b7dfdcc to 17cb126 Compare October 9, 2024 06:41
@newt-sc
Copy link
Collaborator

newt-sc commented Oct 10, 2024

@xZetsubou
Copy link
Contributor Author

That's weird to me it didn't shows fails addi7ted: 11249638769 I'll double check it. same with old tests here 11225330115

Can you re-run the action again

@newt-sc
Copy link
Collaborator

newt-sc commented Oct 10, 2024

I've re-run it 7 times.. no luck

@newt-sc
Copy link
Collaborator

newt-sc commented Oct 10, 2024

Will merge it and see how it goes. Will fix the tests later if necessary.

@newt-sc newt-sc merged commit 29ca9b7 into a4k-openproject:master Oct 10, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants